-
Notifications
You must be signed in to change notification settings - Fork 307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Customizable Dynamic Kaplan-Meier Plot for Survival Analysis #4900
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cbioportalfrontend ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
00258c8
to
51e2fef
Compare
afa3564
to
c4e675c
Compare
ca52e19
to
350f297
Compare
package-lock.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we shouldn't need package lock. use yarn to add new things, not npm
@@ -75,7 +76,13 @@ export default class GroupComparisonPage extends React.Component< | |||
this.pathwayMapperUserSelectionStore = new GroupComparisonPathwayMapperUserSelectionStore(); | |||
this.queryReaction = reaction( | |||
() => this.urlWrapper.query.comparisonId, | |||
sessionId => { | |||
(sessionId, previousSessionId) => { | |||
if (localStorage.getItem('preventPageReload') === 'true') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to understand this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added comment
|
||
const survivalPrefixes = _.map( | ||
this.props.store.survivalTitleByPrefix | ||
.result! as Dictionary<string>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't need cast if type properly
} | ||
|
||
@observable | ||
public startEventPosition: 'FIRST' | 'LAST' = 'FIRST'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this needs to be someone scoped better. i.e. it needs to be somehow associated with the survival plot stuff, not just hanging out here in top level of store.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to new tab survival store
When there is only one _status variable, the table will be hidden and there would be no way to add a dynamic KM plot. e.g. https://deploy-preview-4900.cancerrevue.org/comparison/survival?comparisonId=61b4d97bf8f71021ce57c8bf
When there is no _status variable (no survival data) but there are still timeline data, there is no way currently to add dynamic KM plot.
|
this is now resolved |
Address issue Dynamic KM Plot and RFC 36
This frontend PR aims to have a customizable Dynamic Kaplan-Meier plot that allows users to specify start point and endpoint or censored point for survival analysis.